bpo-42246: Make sure that line number is correct after a return, as required by PEP 626#23495
Conversation
63c5fa2 to
b142622
Compare
gpshead
left a comment
There was a problem hiding this comment.
LGTM though it'd be a good idea to have more eyeballs than just mine on this. 👍
|
@pablogsal would you mind taking a quick look at this? |
Yep, will review this today |
Python/compile.c
Outdated
| static int | ||
| is_exit_without_lineno(basicblock *b) { | ||
| return b->b_exit && b->b_instr[0].i_lineno < 0; | ||
| } |
There was a problem hiding this comment.
| static int | |
| is_exit_without_lineno(basicblock *b) { | |
| return b->b_exit && b->b_instr[0].i_lineno < 0; | |
| } | |
| static inline int | |
| is_exit_without_lineno(basicblock *b) { | |
| return b->b_exit && b->b_instr[0].i_lineno < 0; | |
| } |
Python/compile.c
Outdated
| if (result == NULL) { | ||
| return NULL; | ||
| } | ||
| for(int i = 0; i < block->b_iused; i++) { |
There was a problem hiding this comment.
| for(int i = 0; i < block->b_iused; i++) { | |
| for (int i = 0; i < block->b_iused; i++) { |
Python/compile.c
Outdated
| } | ||
|
|
||
|
|
||
| static int |
There was a problem hiding this comment.
| static int | |
| static inline int |
There was a problem hiding this comment.
I've deleted the function and inlined its body of the function. It was only used in once place.
| ADDOP(c, GET_ITER); | ||
| compiler_use_next_block(c, start); | ||
| ADDOP_JUMP(c, FOR_ITER, cleanup); | ||
| compiler_use_next_block(c, body); |
There was a problem hiding this comment.
Why is this needed? To repeat the "body" if there is an implicit return? If so, could you add an example to the test?
There was a problem hiding this comment.
FOR_ITER is a branch, so we need a new block immediately after it.
https://github.com/python/cpython/pull/23495/files#diff-ebc983d9f91e5bcf73500e377ac65e85863c4f77fd5b6b6caf4fcdf7c0f0b057R6401 checks it.
Python/compile.c
Outdated
| */ | ||
| for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) { | ||
| if (b->b_iused > 0 && is_jump(&b->b_instr[b->b_iused-1])) { | ||
| switch(b->b_instr[b->b_iused-1].i_opcode) { |
There was a problem hiding this comment.
| switch(b->b_instr[b->b_iused-1].i_opcode) { | |
| switch (b->b_instr[b->b_iused-1].i_opcode) { |
|
LGTM in general but I left some minor suggestions |
…equired by PEP 626 (pythonGH-23495) Make sure that line number is correct after a return, as defined by PEP 626.
This PR ensures that
f_linenocan be correctly computed after a return, by ensuring that all compiler generated return instructionshave a line number.
Any block that has no line numbers and has only one predecessor can inherit the line number from its predecessor.
Any block ending in a return has no successors, and thus can be safely duplicated.
By duplicating all blocks that end in a return, but have no line number, those blocks will have only one predecessor and thus can inherit its predecessor's line number, meaning that all implicit returns now have a line number.
For example,
currently compiles to:
with this PR it compiles to:
which is equivalent, but produces the correct line number when
not cond.https://bugs.python.org/issue42246